Added trace ID to error events #251
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lemnik
reviewed
Jul 11, 2024
Comment on lines
413
to
415
| if (correlation != null) { | ||
|
|
||
| String traceId = getString(correlation, "traceId"); |
There was a problem hiding this comment.
nit: formatting
Suggested change
| if (correlation != null) { | |
| String traceId = getString(correlation, "traceId"); | |
| if (correlation != null) { | |
| String traceId = getString(correlation, "traceId"); |
Comment on lines
422
to
424
| long traceIdMostSignificantBits = hexToLong(traceId.substring(0, HEX_LONG_LENGTH)); | ||
| long traceIdLeastSignificantBits = hexToLong(traceId.substring(HEX_LONG_LENGTH)); | ||
| long spanIdAsLong = hexToLong(spanId); |
There was a problem hiding this comment.
we should do some error-catching (NumberFormatException at minimum) here, if (somehow) these aren't exactly what we expect we would crash on the hot-path... if the strings aren't hex encoded we can safely omit the entire correlation element
lemnik
reviewed
Jul 11, 2024
| @@ -442,4 +462,9 @@ JSONObject deliverEvent(@Nullable JSONObject eventJson) { | |||
| boolean hasString(JSONObject args, String key) { | |||
| return getString(args, key) != null; | |||
| } | |||
|
|
|||
| static long hexToLong(String hex) { | |||
There was a problem hiding this comment.
nit: this doesn't appear to be used outside of the BugsnagFlutter class
Suggested change
| static long hexToLong(String hex) { | |
| private static long hexToLong(String hex) { |
lemnik
reviewed
Jul 25, 2024
| event.setTraceCorrelation(new UUID(traceIdMostSignificantBits, traceIdLeastSignificantBits), spanIdAsLong); | ||
| } | ||
| } catch(Exception e) { | ||
| Log.e("BugsnagFlutter", "correlation parsing", e); |
There was a problem hiding this comment.
I think that this can be reasonably dropped, logging to the device log just adds noise. This should never happen after-all.
Suggested change
| Log.e("BugsnagFlutter", "correlation parsing", e); | |
| // ignore the error, the error correlation will be missing |
lemnik
reviewed
Jul 25, 2024
robert-smartbear
force-pushed
the
robert/plat-12234-error-correlation
branch
from
78dc616 to
c596f82
Compare
Added an entry to Changelog. The e2e tests were added in bugsnag-flutter-performance |
lemnik
approved these changes
Jul 25, 2024
Merged
robert-smartbear
added a commit
that referenced
this pull request
Jul 29, 2024
* Added null checks for client in Android code * Improved the behaviour on Android when Bugsnag is started multiple times from different contexts * Bump bugsnag-android to v6. Changed redactedKeys and discardClasses types to RegExp (#249) * Bump bugsnag-android to v6. Changed redactedKeys and discardClasses types to RegExp * Regular expressions sent to native clients now carry RegExp flags * Update features/fixtures/app/lib/scenarios/start_bugsnag_scenario.dart Co-authored-by: Jason <[email protected]> --------- Co-authored-by: Robert <[email protected]> Co-authored-by: Jason <[email protected]> * docs(readme): amended badge URL to scope to main branch only (#241) Co-authored-by: Tom Longridge <[email protected]> * chore(build): update package gemfile (#240) Co-authored-by: Tom Longridge <[email protected]> * Rebuilt Flutter example and fixture with Flutter 3.10 (#252) * Rebuilt flutter example * Rebuilt e2e tests fixture * Small fixes for example project * Replaced examples with a single example for pub.dev release * Small fixes for the fixture * Updated mazerunner gem to v9 * Updated mazerunner to v9 on CI * Switched Android E2E tests to be run on Browserstack on CI * Added browserstack username and access key to docker-compose.yaml * adjust pipeline for browserstack * adjust pipeline for browserstack --------- Co-authored-by: Robert <[email protected]> Co-authored-by: Josh Edney <[email protected]> * Added trace ID to error events (#251) * Added spanId and traceId from span context to events * Updated iOS integration and implemented Android integration for error correlation * Changes requested in code review * Made the package publishable again * Changes requested in code review --------- Co-authored-by: Robert <[email protected]> * Updated UPGRADING.MD for v4 release (#254) * Updated UPGRADING.MD for v4 release * Update UPGRADING.MD Co-authored-by: Tom Longridge <[email protected]> --------- Co-authored-by: Robert <[email protected]> Co-authored-by: Tom Longridge <[email protected]> * Bumped to v4.0.0 --------- Co-authored-by: Robert <[email protected]> Co-authored-by: Jason <[email protected]> Co-authored-by: Tom Longridge <[email protected]> Co-authored-by: Tom Longridge <[email protected]> Co-authored-by: Josh Edney <[email protected]>
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Goal
Include the ID of the current trace and span ID in errors when both bugsnag-flutter and bugsnag-flutter-performance are included in an application
Design
BugsnagContextProvider is used to receive traceId and spanId from Flutter Performance and then the values are included in bugsnag-cocoa and bugsnag-android events
Testing
Existing E2E tests